From 8ee373ad4aaa505310843918ed5a03f9587e41e5 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 1 Jul 2015 10:37:21 -0700 Subject: [PATCH] Add a Target to the build script deps key Previously a Target was not considered to be in the key of the map to build script dependencies, but this meant that information like whether it was a dev-dependency was lost. As a result libraries would depend on the build scripts of their dev-dependencies, causing an internal error when the build script hadn't finished yet when the library was being built. Closes #1751 --- src/cargo/ops/cargo_rustc/context.rs | 2 +- src/cargo/ops/cargo_rustc/custom_build.rs | 20 +++++++++-------- src/cargo/ops/cargo_rustc/mod.rs | 13 ++++++------ tests/test_cargo_test.rs | 26 +++++++++++++++++++++++ 4 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 642942722..26b4a14c0 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -36,7 +36,7 @@ pub struct Context<'a, 'cfg: 'a> { Fingerprint>, pub compiled: HashSet<(&'a PackageId, &'a Target, &'a Profile)>, pub build_config: BuildConfig, - pub build_scripts: HashMap<(&'a PackageId, Kind, &'a Profile), + pub build_scripts: HashMap<(&'a PackageId, &'a Target, &'a Profile, Kind), Vec<&'a PackageId>>, host: Layout, diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index acf9788ae..b2c98a41b 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -103,7 +103,8 @@ pub fn prepare(pkg: &Package, target: &Target, req: Platform, let id = pkg.package_id().clone(); let all = (id.clone(), pkg_name.clone(), build_state.clone(), build_output.clone()); - let plugin_deps = super::load_build_deps(cx, pkg, profile, Kind::Host); + let plugin_deps = super::load_build_deps(cx, pkg, target, profile, + Kind::Host); try!(fs::create_dir_all(&cx.layout(pkg, Kind::Target).build(pkg))); try!(fs::create_dir_all(&cx.layout(pkg, Kind::Host).build(pkg))); @@ -349,7 +350,7 @@ impl BuildOutput { /// targets/profiles which are being built. pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, pkg: &'b Package, - targets: &[(&Target, &'b Profile)]) { + targets: &[(&'b Target, &'b Profile)]) { let mut ret = HashMap::new(); for &(target, profile) in targets { build(&mut ret, Kind::Target, pkg, target, profile, cx); @@ -357,21 +358,22 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, } // Make the output a little more deterministic by sorting all dependencies - for (&(id, kind, _), slot) in ret.iter_mut() { + for (&(id, target, _, kind), slot) in ret.iter_mut() { slot.sort(); slot.dedup(); - debug!("script deps: {}/{:?} => {:?}", id, kind, + debug!("script deps: {}/{}/{:?} => {:?}", id, target.name(), kind, slot.iter().map(|s| s.to_string()).collect::>()); } cx.build_scripts = ret; // Recursive function to build up the map we're constructing. This function // memoizes all of its return values as it goes along. - fn build<'a, 'b, 'cfg>(out: &'a mut HashMap<(&'b PackageId, Kind, &'b Profile), + fn build<'a, 'b, 'cfg>(out: &'a mut HashMap<(&'b PackageId, &'b Target, + &'b Profile, Kind), Vec<&'b PackageId>>, kind: Kind, pkg: &'b Package, - target: &Target, + target: &'b Target, profile: &'b Profile, cx: &Context<'b, 'cfg>) -> &'a [&'b PackageId] { @@ -381,8 +383,8 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, // dependencies. let kind = if target.for_host() {Kind::Host} else {kind}; let id = pkg.package_id(); - if out.contains_key(&(id, kind, profile)) { - return &out[&(id, kind, profile)] + if out.contains_key(&(id, target, profile, kind)) { + return &out[&(id, target, profile, kind)] } // This loop is both the recursive and additive portion of this @@ -416,7 +418,7 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, } } - let prev = out.entry((id, kind, profile)).or_insert(Vec::new()); + let prev = out.entry((id, target, profile, kind)).or_insert(Vec::new()); prev.extend(ret); return prev } diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index db5d0436b..486ce8d5c 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -331,7 +331,7 @@ fn rustc(package: &Package, target: &Target, profile: &Profile, let rustcs = try!(prepare_rustc(package, target, profile, crate_types, cx, req)); - let plugin_deps = load_build_deps(cx, package, profile, Kind::Host); + let plugin_deps = load_build_deps(cx, package, target, profile, Kind::Host); return rustcs.into_iter().map(|(mut rustc, kind)| { let name = package.name().to_string(); @@ -352,7 +352,8 @@ fn rustc(package: &Package, target: &Target, profile: &Profile, let build_state = cx.build_state.clone(); let current_id = package.package_id().clone(); let plugin_deps = plugin_deps.clone(); - let mut native_lib_deps = load_build_deps(cx, package, profile, kind); + let mut native_lib_deps = load_build_deps(cx, package, target, profile, + kind); if package.has_custom_build() && !target.is_custom_build() { native_lib_deps.insert(0, current_id.clone()); } @@ -384,9 +385,9 @@ fn rustc(package: &Package, target: &Target, profile: &Profile, // dynamic library load path as a plugin's dynamic library may be // located somewhere in there. let build_state = build_state.outputs.lock().unwrap(); - add_native_deps(&mut rustc, &*build_state, native_lib_deps, + add_native_deps(&mut rustc, &build_state, native_lib_deps, kind, pass_l_flag, ¤t_id); - try!(add_plugin_deps(&mut rustc, &*build_state, plugin_deps)); + try!(add_plugin_deps(&mut rustc, &build_state, plugin_deps)); drop(build_state); // FIXME(rust-lang/rust#18913): we probably shouldn't have to do @@ -456,10 +457,10 @@ fn rustc(package: &Package, target: &Target, profile: &Profile, } } -fn load_build_deps(cx: &Context, pkg: &Package, +fn load_build_deps(cx: &Context, pkg: &Package, target: &Target, profile: &Profile, kind: Kind) -> Vec { let pkg = cx.get_package(pkg.package_id()); - cx.build_scripts.get(&(pkg.package_id(), kind, profile)).map(|deps| { + cx.build_scripts.get(&(pkg.package_id(), target, profile, kind)).map(|deps| { deps.iter().map(|&d| d.clone()).collect::>() }).unwrap_or(Vec::new()) } diff --git a/tests/test_cargo_test.rs b/tests/test_cargo_test.rs index 91304272a..268355179 100644 --- a/tests/test_cargo_test.rs +++ b/tests/test_cargo_test.rs @@ -1631,3 +1631,29 @@ test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured ", compiling = COMPILING, running = RUNNING, doctest = DOCTEST))) }); + +test!(dev_dep_with_build_script { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dev-dependencies] + bar = { path = "bar" } + "#) + .file("src/lib.rs", "") + .file("examples/foo.rs", "fn main() {}") + .file("bar/Cargo.toml", r#" + [package] + name = "bar" + version = "0.0.1" + authors = [] + build = "build.rs" + "#) + .file("bar/src/lib.rs", "") + .file("bar/build.rs", "fn main() {}"); + assert_that(p.cargo_process("test"), + execs().with_status(0)); +}); -- 2.30.2